Skip to content

fix(server): restrict SQLite database file permissions to 0o600#1359

Merged
TaylorMutch merged 1 commit into
NVIDIA:mainfrom
alangou:alangou/os-79-fsr-029-restrict-sqlite-database-file-permissions
May 13, 2026
Merged

fix(server): restrict SQLite database file permissions to 0o600#1359
TaylorMutch merged 1 commit into
NVIDIA:mainfrom
alangou:alangou/os-79-fsr-029-restrict-sqlite-database-file-permissions

Conversation

@alangou
Copy link
Copy Markdown
Contributor

@alangou alangou commented May 13, 2026

Summary

The gateway SQLite database stores provider API keys, SSH session tokens, and sandbox metadata, but SQLite was creating it with 0o644 & ~umask — readable by other local users on shared hosts. This PR tightens the on-disk mode to 0o600 immediately after the pool is connected, and applies the same hardening to the <db>-wal and <db>-shm sidecars when SQLite is in WAL mode. Satisfies FSR-029 (mitigates T-038).

Related Issue

Closes OS-79

Changes

  • crates/openshell-server/src/persistence/sqlite.rs:
    • Capture the database filename before connect_with consumes the options, then tighten permissions to 0o600 once the pool is up. The hardening is reapplied on every connect so pre-existing databases from older gateway versions are also locked down. In-memory databases (:memory: / mode=memory) are skipped.
    • Delegate the chmod to the existing openshell_core::paths::set_file_owner_only helper (single source of truth, already cfg(unix)-gated and used by the bootstrap and certgen modules) rather than reimplementing std::fs::set_permissions(..., 0o600).
    • Also tighten the <db>-wal (write-ahead log) and <db>-shm (shared memory index) sidecars when present. They mirror the same sensitive payload as the main file and are created automatically when the gateway runs SQLite in WAL mode. A small sqlite_sidecar_paths helper derives the sidecar paths the same way SQLite does.
  • crates/openshell-server/src/persistence/tests.rs: five cfg(unix) tests covering the hardening at increasing levels of fidelity:
    • sqlite_connect_restricts_db_file_permissions — fresh DB created via Store::connect ends up at 0o600.
    • sqlite_connect_tightens_existing_db_file_permissions — pre-existing DB at 0o644 (older gateway version) is re-tightened on reconnect.
    • restrict_db_file_permissions_tightens_main_and_wal_and_shm_files — synthetic main + WAL + SHM files at 0o644 are all tightened to 0o600 (proves the chmod loop walks all three paths).
    • restrict_db_file_permissions_skips_missing_sidecars — the exists() guard avoids errors when no WAL/SHM sidecar is present (the current production path; sqlx 0.8 does not default to WAL and does not parse journal_mode from the URL).
    • restrict_db_file_permissions_handles_real_sqlite_wal_files — opens a real sqlx pool with SqliteJournalMode::Wal via the builder API, forces a write so SQLite materializes the WAL on disk, loosens permissions, then verifies the helper tightens the main DB plus both real sqlite-managed sidecars back to 0o600.
  • architecture/gateway.md: updated the Persistence section to document the on-disk mode invariant for both the main DB file and the <db>-wal / <db>-shm sidecars.

Testing

  • mise run pre-commit passes (cargo fmt, clippy -D warnings, license headers)
  • Unit tests added/updated — five #[cfg(unix)] tests listed above; full persistence:: suite (38 tests) green
  • E2E tests added/updated (if applicable) — N/A, no changes under e2e/

Manual verification matches the issue's acceptance criterion: ls -la <db>* shows -rw------- for the database and any WAL/SHM sidecars after the gateway connects.

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

@alangou alangou force-pushed the alangou/os-79-fsr-029-restrict-sqlite-database-file-permissions branch from c41be50 to 61bcf0d Compare May 13, 2026 15:21
Signed-off-by: Adrien Langou <alangou@nvidia.com>
@alangou alangou force-pushed the alangou/os-79-fsr-029-restrict-sqlite-database-file-permissions branch from 61bcf0d to 748f712 Compare May 13, 2026 15:36
@TaylorMutch TaylorMutch added the test:e2e Requires end-to-end coverage label May 13, 2026
@github-actions
Copy link
Copy Markdown

Label test:e2e applied for 748f712. Open the existing run and click Re-run all jobs to execute with the label set. The E2E Gate check on this PR will flip green automatically once the run finishes.

Copy link
Copy Markdown
Collaborator

@TaylorMutch TaylorMutch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@TaylorMutch TaylorMutch merged commit 5159ebc into NVIDIA:main May 13, 2026
44 of 45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants